Skip to content

feat(select): allow HTML within options#31072

Open
thetaPC wants to merge 19 commits intonextfrom
FW-7137
Open

feat(select): allow HTML within options#31072
thetaPC wants to merge 19 commits intonextfrom
FW-7137

Conversation

@thetaPC
Copy link
Copy Markdown
Contributor

@thetaPC thetaPC commented Apr 9, 2026

Issue number: resolves #29890


What is the current behavior?

Select only allows plain text to be passed within it's options.

What is the new behavior?

ion-select-option

  • Added default slot support for custom HTML content (images, avatars, icons, etc.) when innerHTMLTemplatesEnabled is true
  • Added start and end named slots for content that appears in the overlay interface but not in the selected text
  • Added description prop for text rendered below the option label in the overlay

ion-select

  • Selected text renders HTML content from the default slot, excluding start/end slot content
  • aria-label derives plain text only from the default slot, ensuring screen readers don't read slotted or element content
  • Added CSS variables for styling media within the selected text (--select-text-media-width, --select-text-media-height, etc.)

Overlay interfaces (alert, action-sheet, select-popover, select-modal)

  • All four interfaces render rich content (start, end, description, HTML label) consistently via the shared renderOptionLabel utility
  • Public interfaces (ActionSheetButton, AlertInput, SelectPopoverOption, SelectModalOption) are unchanged — rich content fields are on internal extended interfaces (SelectActionSheetButton, SelectAlertInput, SelectOverlayOption)
  • Content is sanitized via sanitizeDOMString before DOM injection to prevent XSS

Utilities

  • Added select-option-render.tsx: shared render utility for option labels across all overlay components
  • Added getOptionContent: extracts and clones slot content from ion-select-option for overlays and selected text
  • Added getDefaultSlotPlainText — extracts plain text from the default slot, used for labels and aria

Tests

  • Added E2E tests for rich content rendering across all four interfaces (single and multiple selection)
  • Added tests for slot positioning (start/end placement verification)
  • Added tests for selected text content verification (excludes slotted content)

Does this introduce a breaking change?

  • Yes
  • No

No, developers will be able to continue using plain text for select options as usual.

Other information

Preview

@github-actions github-actions bot added the package: core @ionic/core package label Apr 9, 2026
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview, Comment Apr 15, 2026 11:20pm

Request Review

// --------------------------------------------------

.action-sheet-button-label {
gap: 12px;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to use the same value that ionic uses.

<div class="alert-button-inner">
<div class="alert-checkbox-icon">
<div class="alert-checkbox-inner"></div>
{inputs.map((i) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change is the addition of optionLabelOptions and renderOptionLabel

<div class="alert-button-inner">
<div class="alert-radio-icon">
<div class="alert-radio-inner"></div>
{inputs.map((i) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change is the addition of optionLabelOptions and renderOptionLabel

*/
this.closeModal();
}
{this.options.map((option, index) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change is the addition of optionLabelOptions and renderOptionLabel

onIonChange={(ev) => {
this.setChecked(ev);
this.callOptionHandler(ev);
return this.options.map((option, index) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change is the addition of optionLabelOptions and renderOptionLabel

/**
* Text that is placed underneath the option text to provide additional details about the option.
*/
@Prop() description?: string;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description isn't being used here to display on the screen because ion-select-option isn't used to display the options, it's done through the respective interface. So description is passed to the interface.

Comment on lines +46 to +48
<slot name="start"></slot>
<slot></slot>
<slot name="end"></slot>
Copy link
Copy Markdown
Contributor Author

@thetaPC thetaPC Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slots aren't being used here to display on the screen because ion-select-option isn't used to display the options, it's done through the respective interface. So slots are passed to the interface and the rendering is done with a new utility.

Even though it's not used to display here, it's still useful to include here so the docs can generate that "slots" are available for the options.

onIonChange={(ev) => {
this.setChecked(ev);
this.callOptionHandler(ev);
return options.map((option, index) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change is the addition of optionLabelOptions and renderOptionLabel

*/
this.dismissParentPopover();
}
{options.map((option, index) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change is the addition of optionLabelOptions and renderOptionLabel

@thetaPC thetaPC marked this pull request as ready for review April 11, 2026 02:06
@thetaPC thetaPC requested a review from a team as a code owner April 11, 2026 02:06
@thetaPC thetaPC requested review from BenOsodrac and ShaneK and removed request for BenOsodrac April 11, 2026 02:06
Copy link
Copy Markdown
Member

@ShaneK ShaneK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good! Just some feedback on some things I noticed


export interface ActionSheetButton<T = any> {
text?: string;
text?: string | HTMLElement;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Widening text from string to string | HTMLElement here affects all action-sheet consumers, not just select-driven ones. Same with AlertInput.label. Anyone calling actionSheetController.create() who does button.text.toLowerCase() or passes text to a string-typed function will get TS errors. Since this targets next, breaking changes are expected, but would it be cleaner to add a separate property (e.g., richText or contentElement) for the HTMLElement case? That way existing string usage stays untouched.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point! cc0af05 and 49e0e6c

<span class="action-sheet-button-inner">
{b.icon && <ion-icon icon={b.icon} aria-hidden="true" lazy={false} class="action-sheet-icon" />}
{b.text}
{renderOptionLabel(optionLabelOptions, 'action-sheet-button-label')}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the DOM structure for all action-sheet buttons, not just ones created through select. Previously {b.text} rendered directly inside .action-sheet-button-inner, now it's wrapped in <span class="action-sheet-button-label"><span>...</span></span>. Same thing happens in alert for radio/checkbox labels. Any app CSS targeting these internal elements will break. Should renderOptionLabel only wrap when rich content is actually present (when startContent/endContent/description exist), falling back to the original rendering for plain strings?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do understand that breaking changes are acceptable here and this may be intended. Just double checking that's what you're going for.

Copy link
Copy Markdown
Contributor Author

@thetaPC thetaPC Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for the extra wrappers so it's now back to plain: 0c7e4d6 and 17e9e37

Comment thread core/src/utils/select-option-render.tsx Outdated

// Clear previous content and clone new nodes
el.innerHTML = '';
Array.from(content.childNodes).forEach((n) => el.appendChild(n.cloneNode(true)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The select's closed-state display goes through sanitizeDOMString before innerHTML injection, but this overlay path clones nodes with cloneNode(true) without sanitization. Inline event handlers (onclick, onerror) on elements inside ion-select-option get faithfully cloned into the overlay DOM. The two rendering paths for the same content have different security postures.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<script>
window.Ionic = {
config: {
innerHTMLTemplatesEnabled: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This window.Ionic = { config: { innerHTMLTemplatesEnabled: true } } replaces the entire config object that setContent already sets in the head (which includes mode and theme). I think it works by accident because md is the fallback on non-iOS platforms, but you're losing the explicit config. Should be merging instead: window.Ionic = window.Ionic || {}; window.Ionic.config = { ...window.Ionic.config, innerHTMLTemplatesEnabled: true };

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved them to their own page: 8a4e59b

},
startContent: startContent ?? undefined,
endContent: endContent ?? undefined,
description: option.description,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're gating startContent/endContent behind this.customHTMLEnabled but description gets passed through regardless. Is that intentional? If someone has innerHTMLTemplatesEnabled set to false (the default), they'd still see descriptions in overlays but not the slot content. Feels like it should be consistent either way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was intentional. Devs can only pass string text as a description. To me, it would feel weird that a dev would have to enable innerHTMLTemplatesEnabled when they only want to have a description (no start or end slots). It follows how other props are being handled.

* spacing between the items without changing the display to prevent
* losing the ellipsis behavior.
*/
.select-text > * {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.select-text > * applies margin-inline-start to the first child too, which adds unwanted leading space. Should be .select-text > * + * or .select-text > *:not(:first-child) to only space between items. Same thing in the ionic theme file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up finding an issue with my approach and the proposed approached when it came to multiple values and the rich content option is second on the list. The styling isn't applied as intended so there's no spacing between the plain text and the rich content. I ended wrapping everyone in a span if it has rich content and if it's in the select text: f2695ca

// Ionic Select Popover
// --------------------------------------------------

// Select Modal: Select Option
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This says "Select Modal" but it's the select-popover file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<script>
window.Ionic = {
config: {
innerHTMLTemplatesEnabled: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting innerHTMLTemplatesEnabled: true globally here means all existing tests that navigate to this page now run with a different config than they were written for. Could the existing overlay open/dismiss and focus tests be affected? Would it be safer to create a separate test page for the rich content tests, or set the config per-test in page.setContent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it to their own page: 8a4e59b

// --------------------------------------------------

.action-sheet-button-label {
gap: 12px;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based it off ionic theme

@thetaPC thetaPC requested a review from ShaneK April 16, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: angular @ionic/angular package package: core @ionic/core package package: vue @ionic/vue package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants